Fix character-range highlights and retain debugLineFragments - #26

Thanks for the follow-up! 🙏 This is in good shape. Every point from the #19 review is addressed. I also checked that both strengthened tests actually catch regressions: with mergeGroup++ removed, the image test fails (one 0→208 rect), and with the isPreformatted guard removed, the pre test fails (80 vs 80). I merged this locally with current main (1.12.0). There are no conflicts, the full root+core suite passes (2526), and analyze is clean.

One thing before merge, in the ellipsis part of debugLineFragments():

ellipsisVisibleLength is always null on the fragment that's actually on the line. When layout truncates, it adds a new truncFrag ("visi…") to the line. But it sets ellipsisVisibleLength on the original fragment, which never reaches _lines (render_hyper_box_layout.dart ~L1322–1335). I reproduced it with:

<p>x</p><div style="overflow:hidden;text-overflow:ellipsis;white-space:nowrap">visible head SECRET TAIL</div>

at width 80. The line fragment reports text: "visi…", charStart: 1, charEnd: 6, ellipsisVisibleLength: null. The expected result is 4 visible chars, so charEnd: 5. The … glyph gets counted as source char 5. That causes two problems:

• The doc comment says consumers "can tell how much of a truncated fragment reached the screen", but they can't.
• getBoxesForCharRange(5, 6) returns a rect over the … for the hidden char b.

The current test only checks containsKey('ellipsisVisibleLength'), so it can't catch this.

Suggested fix:

1. In layout, set ellipsisVisibleLength = clippedText.length on truncFrag. Do the same for the ellipsis-only fragment, with 0.
2. In debugLineFragments() and getBoxesForCharRange(), use fragment.ellipsisVisibleLength ?? text.length as the fragment's char length. getSelectedText already does this.
3. Add a test with the HTML above that asserts ellipsisVisibleLength == 4, charEnd == 5, and getBoxesForCharRange(5, 6) is empty. The same harness exists in test/review_fixes_v1_3_2_test.dart.

Small stuff (non-blocking):

• The image test comment mentions commit f5a313b, which no longer exists after the rebase. Could you drop the hash?
• Could you add a line to the getBoxesForCharRange doc saying offsets are local to this RenderHyperBox? In virtualized/auto mode (>10k chars), each chunk has its own box, and offsets restart at 0.
• Optional: most of the per-fragment logic is duplicated from getSelectionRects. A shared helper would be nice, but it's fine to leave as a follow-up.

Thanks again, nearly there!

Thanks for the quick turnaround! 🙏 Everything from the last review is addressed.

I checked 5a151ff locally merged with current main (1.12.0). There are no conflicts, analyze is clean, format is unchanged, and the full root+core suite passes (2527, which is the old count plus your new test). I also reverted just the truncFrag change, and the new test fails (Expected: <4>, Actual: <null>), so it really guards the fix. The shared _getFragmentBoxesForRange helper is a nice cleanup, and getSelectionRects keeps its old trimming behavior.

Two small behavior changes come with this. Both look right to me, I'm just noting them for the record:

• else if → if in the ellipsis path: when fitWidth > 0 but no characters fit on an empty line, we now show … instead of an empty line.
• Selection highlights on a truncated fragment no longer cover the … glyph, which matches getSelectedText.

Merging now. Thanks again for sticking with this through two rounds! 🙌